-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http: remove default 'drain' listener on upgrade #18866
Conversation
test/parallel/test-http-connect.js
Outdated
@@ -62,6 +62,7 @@ server.listen(0, common.mustCall(() => { | |||
assert(!socket.onend); | |||
assert.strictEqual(socket.listeners('connect').length, 0); | |||
assert.strictEqual(socket.listeners('data').length, 0); | |||
assert.strictEqual(socket.listenerCount('drain'), 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these could be kept consistent if we want to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I actually wanted to change all of them to listenerCount
but that's a perfect code+learn exercise, I guess.
Ensure that the default `'drain'` listener is removed before the `'connect'` or `'upgrade'` event is emitted.
c7e5a54
to
575da74
Compare
New CI before landing: https://ci.nodejs.org/job/node-test-pull-request/13329/ |
Landed in 2b7f920 🎉 |
Ensure that the default `'drain'` listener is removed before the `'connect'` or `'upgrade'` event is emitted. PR-URL: nodejs#18866 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Ensure that the default `'drain'` listener is removed before the `'connect'` or `'upgrade'` event is emitted. PR-URL: nodejs#18866 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Ensure that the default `'drain'` listener is removed before the `'connect'` or `'upgrade'` event is emitted. PR-URL: #18866 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Ensure that the default `'drain'` listener is removed before the `'connect'` or `'upgrade'` event is emitted. PR-URL: #18866 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Ensure that the default `'drain'` listener is removed before the `'connect'` or `'upgrade'` event is emitted. PR-URL: nodejs#18866 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Ensure that the default
'drain'
listener is removed before the'connect'
or'upgrade'
event is emitted.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
http